UIDATIMP-1540: Fetch Job summary data with for a particular tenant id #1478
Conversation
There was a problem hiding this comment.
LGTM, but what do you think about just implementing useTenantKy in useOkapiKy directly in stripes-core? useTenantKy feels extraneous given that tenantId is implemented as an optional prop.
Aside, note that this value is tenant rather than tenantId in the rest of the stripes codebase and in stripes.config.js. Is it really named tenantId in the jobLogData responses? If so, that is exceptionally exceptionally unfortunate. From the stripes point of view, I'd say we should call it tenant in the hook and rename it to tenant when passing it into all those use...Query() hooks, but that introduces inconsistency with the property on those data structures. Where do you think consistency is most important?
src/hooks/useTenantKy.js
Outdated
There was a problem hiding this comment.
Do you think this hook belongs here in DI or could/should we just tweak useOkapiKy in stripes-core to accomodate requests that want to override the tenant value from stripes?
There was a problem hiding this comment.
Hi @zburke I think adjusting useOkpapiKy in stripes-core would be really nice, because not only DI module overrides headers but also other modules that are implementing ECS stuff (like ui-inventory/ui-consortia-settings). I just didn't want to touch stripes-core because of upcoming releases deadline.
There was a problem hiding this comment.
Regarding renaming tenantId to tenant I'll discuss it with BE devs and rename this arg in hooks
There was a problem hiding this comment.
Created a PR in stripes-core - folio-org/stripes-core#1348
There was a problem hiding this comment.
I brought up tenant vs tenantId with BE folks already and was basically told, "This is already a mess in other places. Changing it here won't actually make the whole system less messy, it'll just make it less messy for you. Since our work is already done and tenantId is a better name anyway, we're not gonna do it again. Sorry not sorry." 😒
There was a problem hiding this comment.
yep, I noticed that mod-search also uses tenantId prop e.g. But I guess we could be at least consistent on UI and use the same naming as it is in stripes.config
mariia-aloshyna
force-pushed
the
UIDATIMP-1540
branch
from
696e2c4 to
559b1b2
Compare
|
Kudos, SonarCloud Quality Gate passed! |
Purpose
To provide marc record and instance on the member tenant log page with jsons when marc-to-marc matching and marc-bib update has been performed on the central tenant by data import initiated on a member tenant.
Approach
useTenantKyhook to extend headers with tenantIdRefs
https://issues.folio.org/browse/UIDATIMP-1540
Screenshots